Skip to content

Python: Fix duplicate tool result messages in handoff workflow #2712

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
lbbniu wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Python: Fix duplicate tool result messages in handoff workflow #2712

lbbniu wants to merge 3 commits into from

Conversation

@lbbniu
Copy link
Contributor

@lbbniu lbbniu commented Dec 9, 2025

Fixes #2711

Summary

When using HandoffBuilder, the _HandoffCoordinator._append_tool_acknowledgement method was creating duplicate tool result messages in the conversation history. This occurred because the method didn't check if a tool result with the same call_id already existed before appending a new one.

Problem

The duplication happened when:

  1. _AutoHandoffMiddleware creates a synthetic tool result
  2. ChatAgent appends this result to the conversation
  3. _HandoffCoordinator._append_tool_acknowledgement creates another result with the same call_id but different author_name

This led to:

  • Polluted conversation history
  • Unnecessary token usage
  • Increased checkpoint storage
  • Confusing debugging experience

Changes

  • Add _has_tool_result_for_call() module-level helper function to check if a tool result with the given call_id already exists in the conversation
  • Modify _append_tool_acknowledgement() to skip adding tool result if one already exists with the same call_id
  • Add debug logging when skipping duplicate tool acknowledgement

Testing

  • ✅ All handoff tests pass (20/20)
  • ✅ All checkpoint tests pass (47/47)

Copilot AI review requested due to automatic review settings December 9, 2025 03:36
Copilot AI reviewed Dec 9, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a bug where duplicate tool result messages were being created in the conversation history when using HandoffBuilder. The fix adds deduplication logic to prevent the _HandoffCoordinator from appending a tool acknowledgement when one already exists with the same call_id.

  • Introduces a helper function to check for existing tool results with the same call_id
  • Modifies the tool acknowledgement logic to skip duplicates created by _AutoHandoffMiddleware
  • Adds debug logging when skipping duplicate tool acknowledgements

moonbox3
moonbox3 approved these changes Dec 9, 2025
View reviewed changes
Copy link
Contributor

@moonbox3 moonbox3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing, @lbbniu

@markwallace-microsoft
Copy link
Member

markwallace-microsoft commented Dec 9, 2025 •
edited
Loading

Python Test Coverage

Python Test Coverage Report •
FileStmtsMissCoverMissing
packages/core/agent_framework/_workflows
   _handoff.py62614277%63, 76–78, 85–86, 88, 90, 204, 212–217, 220–221, 235, 240, 259–262, 271–273, 284, 287, 297–308, 310, 316, 322, 378, 394, 396, 399, 401, 450, 559, 568–573, 575–576, 587, 612, 630, 645, 657, 679, 691–693, 699, 742–744, 747–750, 752–754, 1116, 1121, 1125, 1130, 1205, 1215, 1308, 1311, 1327, 1332, 1344, 1350–1353, 1396–1397, 1536, 1818, 1830, 1853, 1859, 1868, 1872, 1877, 1897, 1919, 1930, 1942, 1975, 1986, 1990, 1992–1996, 2012–2014, 2016–2023, 2025–2027, 2029–2030, 2032, 2034, 2054–2060, 2062, 2068
TOTAL16833261884% 

Python Unit Test Overview

Tests Skipped Failures Errors Time
2531 148 💤 0 ❌ 0 🔥 1m 0s ⏱️

@moonbox3
Copy link
Contributor

moonbox3 commented Dec 15, 2025

@lbbniu please make sure the unit tests are passing locally, as well as the pre-commit.

@lbbniu
Copy link
Contributor Author

lbbniu commented Dec 17, 2025 •
edited
Loading

@lbbniu please make sure the unit tests are passing locally, as well as the pre-commit.

OK
image

@lbbniu lbbniu force-pushed the fix/issue-2711-handoff-duplicate-tool-results branch from 5b9252a to 0f70953 Compare December 17, 2025 07:35
@eavanvalkenburg
Copy link
Member

eavanvalkenburg commented Dec 18, 2025

The pre-commit flagged one thing:

SIM102 Use a single `if` statement instead of nested `if` statements
   --> agent_framework/_workflows/_handoff.py:340:13
    |
339 |           for content in msg.contents:
340 | /             if isinstance(content, FunctionResultContent):
341 | |                 if content.call_id == call_id:
    | |______________________________________________^
342 |                       return True
    |
help: Combine `if` statements using `and`

Please make sure you have everything setup, to prevent this!

@lbbniu
Copy link
Contributor Author

lbbniu commented Dec 19, 2025

image

auto-merge was automatically disabled December 19, 2025 03:43

Head branch was pushed to by a user without write access

@lbbniu
Python: Fix duplicate tool result messages in handoff workflow (micro…
89c3ecc 
…soft#2711)

When using HandoffBuilder, the _HandoffCoordinator._append_tool_acknowledgement
method was creating duplicate tool result messages in the conversation history.
This occurred because the method didn't check if a tool result with the same
call_id already existed before appending a new one.

The duplication happened when:
1. _AutoHandoffMiddleware creates a synthetic tool result
2. ChatAgent appends this result to the conversation
3. _HandoffCoordinator._append_tool_acknowledgement creates another result
   with the same call_id but different author_name

This led to:
- Polluted conversation history
- Unnecessary token usage
- Increased checkpoint storage
- Confusing debugging experience

Changes:
- Add _has_tool_result_for_call() module-level helper function to check if
  a tool result with the given call_id already exists in the conversation
- Modify _append_tool_acknowledgement() to skip adding tool result if one
  already exists with the same call_id
- Add debug logging when skipping duplicate tool acknowledgement

Fixes microsoft#2711
@lbbniu
Python: Add test coverage for duplicate tool result detection in hand…
32a7a8a 
…off workflow

This commit addresses Copilot's review comment on PR microsoft#2711 by adding comprehensive
test coverage for the duplicate detection logic introduced in commit d2e9776.

Test coverage includes:
1. Verification that _has_tool_result_for_call() correctly detects existing tool results
2. Verification that _has_tool_result_for_call() returns False for non-existent call_ids
3. Verification that _append_tool_acknowledgement() skips adding duplicates
4. Verification that debug log messages are emitted when duplicates are detected
5. Verification that tool results are added when no duplicate exists

The test ensures that the fix for issue microsoft#2711 (preventing duplicate tool result
messages in the conversation history) remains effective and prevents regression.

# Conflicts:
#	python/packages/core/tests/workflow/test_handoff.py
@lbbniu
Python: Refactor nested if statements in handoff workflow
d913411 
Simplify code by combining nested if statements into a single condition
using 'and' operator. This addresses SIM102 linting issue while maintaining
the same logic for checking tool result call IDs.
@lbbniu lbbniu force-pushed the fix/issue-2711-handoff-duplicate-tool-results branch from c367e0c to d913411 Compare December 19, 2025 03:48
@moonbox3 moonbox3 enabled auto-merge December 19, 2025 04:06
@moonbox3 moonbox3 disabled auto-merge December 19, 2025 04:10
@moonbox3
Copy link
Contributor

moonbox3 commented Dec 19, 2025

@lbbniu do you use VSCode as your IDE? If so, you can run a task called "Run checks." This will run the pre-commit checks. Make sure the pre-commit checks are installed, which is called out in this DEV_SETUP.md:

# Install Python 3.10, 3.11, 3.12, and 3.13
uv python install 3.10 3.11 3.12 3.13
# Create a virtual environment with Python 3.10 (you can change this to 3.11, 3.12 or 3.13)
$PYTHON_VERSION = "3.10"
uv venv --python $PYTHON_VERSION

# NOTE: you may need to activate your virtual environment 

# Install AF and all dependencies
uv sync --dev
# Install all the tools and dependencies
uv run poe install
# Install pre-commit hooks
uv run poe pre-commit-install # < --- PRE-COMMIT INSTALLED HERE

@lbbniu
Copy link
Contributor Author

lbbniu commented Dec 22, 2025

@lbbniu do you use VSCode as your IDE? If so, you can run a task called "Run checks." This will run the pre-commit checks. Make sure the pre-commit checks are installed, which is called out in this DEV_SETUP.md:

# Install Python 3.10, 3.11, 3.12, and 3.13
uv python install 3.10 3.11 3.12 3.13
# Create a virtual environment with Python 3.10 (you can change this to 3.11, 3.12 or 3.13)
$PYTHON_VERSION = "3.10"
uv venv --python $PYTHON_VERSION

# NOTE: you may need to activate your virtual environment 

# Install AF and all dependencies
uv sync --dev
# Install all the tools and dependencies
uv run poe install
# Install pre-commit hooks
uv run poe pre-commit-install # < --- PRE-COMMIT INSTALLED HERE
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@eavanvalkenburg eavanvalkenburg eavanvalkenburg approved these changes

@moonbox3 moonbox3 moonbox3 approved these changes

Assignees

No one assigned

Labels

python

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Python: Duplicate Tool Result Messages in Handoff Workflow

4 participants

@lbbniu @markwallace-microsoft @moonbox3 @eavanvalkenburg